Skip to content

fix(typing): Add overloads for ibis lit#2972

Merged
dangotbanned merged 8 commits intomainfrom
ibis-typing-fixes-1
Aug 14, 2025
Merged

fix(typing): Add overloads for ibis lit#2972
dangotbanned merged 8 commits intomainfrom
ibis-typing-fixes-1

Conversation

@dangotbanned
Copy link
Copy Markdown
Member

What type of PR is this? (check all applicable)

  • 💾 Refactor
  • ✨ Feature
  • 🐛 Bug Fix
  • 🔧 Optimization
  • 📝 Documentation
  • ✅ Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

This is an example of a small, targeted upstream PR we could do.

For now, it covers a lot of our cases and makes the backend code easier to read - without needing to wait on acceptance elsewhere

@dangotbanned dangotbanned added internal typing ibis Issue is related to ibis backend labels Aug 11, 2025
@overload
def lit(value: str, dtype: None = ...) -> ir.StringScalar: ...
@overload
def lit(value: PythonLiteral | ir.Value, dtype: None = ...) -> ir.Scalar: ...
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only included ir.Value here since our typing has a lot of PythonLiteral | ir.Value that doesn't get narrowed any further than that

@dangotbanned dangotbanned marked this pull request as ready for review August 11, 2025 18:45
Copy link
Copy Markdown
Member

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dangotbanned I only have one question :)

Comment on lines +33 to +36
@overload
def lit(value: Literal[-1, 0, 1, 2, 3], dtype: None = ...) -> ir.IntegerScalar: ...
@overload
def lit(value: float, dtype: None = ...) -> ir.FloatingScalar | ir.IntegerScalar: ...
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: I know nothing about ibis

Question: why wouldn't the intuitive mapping work? I tried locally and have no issue 🤔

Suggested change
@overload
def lit(value: Literal[-1, 0, 1, 2, 3], dtype: None = ...) -> ir.IntegerScalar: ...
@overload
def lit(value: float, dtype: None = ...) -> ir.FloatingScalar | ir.IntegerScalar: ...
@overload
def lit(value: int, dtype: None = ...) -> ir.IntegerScalar: ...
@overload
def lit(value: float, dtype: None = ...) -> ir.FloatingScalar: ...

Copy link
Copy Markdown
Member Author

@dangotbanned dangotbanned Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right so both this one and (#2972 (comment)) are to do with overlapping overloads

I might be able to do a repro later, but the general idea is:

  • in python's typing float means int | float (because reasons 😭)
  • in python's runtime, bool subclasses int (because PEP 285)

So I was trying to make use of Literal to avoid overlaps

Literal[-1, 0, 1, 2, 3] just so happened to cover all of our usage 😂

Ideally we'd have the more reasonable:

@overload
def lit(value: bool, dtype: None = ...) -> ir.BooleanScalar: ...
@overload
def lit(value: int, dtype: None = ...) -> ir.IntegerScalar: ...
@overload
def lit(value: float, dtype: None = ...) -> ir.FloatingScalar: ...

Admittedly, I went straight to using Literal, as pyarrow-stubs don't do that - and it ends up bool being matched everywhere 😢

Which I tried to fix in (zen-xu/pyarrow-stubs#209)

image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarcoGorelli have you run into this issue with all your work on the numpy stubs?

A lot of the discussion I've seen on the int | float thing usually mention numpy-like use cases being a challenge

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the overload you put first will get matched first, so you can ignore the overload overlap (like we do for DataFrame.__getitem__ with str vs Sequence[str], IIRC)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be able to do a repro later

Just finished putting this together, and it looks like we don't need to worry about this for ibis 😱

The problem is mainly with mypy, but I forgot that it doesn't understand ibis to begin with 😄:

Show HUGE REPRO narwhals/_ibis/t.py

from __future__ import annotations

import datetime as dt
from typing import TYPE_CHECKING, Any, Literal, overload

import ibis
from typing_extensions import reveal_type

if TYPE_CHECKING:
    import ibis.expr.types as ir
    from typing_extensions import TypeAlias

    from narwhals.typing import PythonLiteral

Incomplete: TypeAlias = Any
"""Marker for upstream issues."""


@overload
def lit(value: Literal[False, True], dtype: None = ...) -> ir.BooleanScalar: ...
@overload
def lit(value: Literal[-1, 0, 1, 2, 3], dtype: None = ...) -> ir.IntegerScalar: ...
@overload
def lit(value: float, dtype: None = ...) -> ir.FloatingScalar | ir.IntegerScalar: ...
@overload
def lit(value: str, dtype: None = ...) -> ir.StringScalar: ...
@overload
def lit(value: PythonLiteral | ir.Value, dtype: None = ...) -> ir.Scalar: ...
@overload
def lit(value: Any, dtype: Any) -> Incomplete: ...
def lit(value: Any, dtype: Any | None = None) -> Incomplete:
    """Alias for `ibis.literal`."""
    literal: Incomplete = ibis.literal
    return literal(value, dtype)


@overload
def lit_intuitive(value: bool, dtype: None = ...) -> ir.BooleanScalar: ...  # noqa: FBT001
@overload
def lit_intuitive(value: int, dtype: None = ...) -> ir.IntegerScalar: ...
@overload
def lit_intuitive(value: float, dtype: None = ...) -> ir.FloatingScalar: ...
@overload
def lit_intuitive(value: str, dtype: None = ...) -> ir.StringScalar: ...
@overload
def lit_intuitive(value: PythonLiteral | ir.Value, dtype: None = ...) -> ir.Scalar: ...
@overload
def lit_intuitive(value: Any, dtype: Any) -> Incomplete: ...
def lit_intuitive(value: Any, dtype: Any | None = None) -> Incomplete:
    literal: Incomplete = ibis.literal
    return literal(value, dtype)


def here_we_go() -> None:  # noqa: PLR0914, PLR0915
    # ruff: noqa: E741

    a: Literal[False] = False
    b: Literal[True] = True
    c: Literal[False, True] = True
    d: bool = False
    e: Literal[0] = 0
    f: Literal[1] = 1
    g: Literal[-1, 0, 1, 2, 3] = -1
    h: int = 1
    i: int = 4
    j: float = 0.0
    k: str = "k"
    l: PythonLiteral = 1
    m: dt.date = dt.date(2000, 1, 1)
    n: Any = 0

    a_1 = lit(a)
    a_2 = lit_intuitive(a)
    reveal_type(a_1), reveal_type(a_2)

    b_1 = lit(b)
    b_2 = lit_intuitive(b)
    reveal_type(b_1), reveal_type(b_2)

    c_1 = lit(c)
    c_2 = lit_intuitive(c)
    reveal_type(c_1), reveal_type(c_2)

    d_1 = lit(d)
    d_2 = lit_intuitive(d)
    reveal_type(d_1), reveal_type(d_2)

    e_1 = lit(e)
    e_2 = lit_intuitive(e)
    reveal_type(e_1), reveal_type(e_2)

    f_1 = lit(f)
    f_2 = lit_intuitive(f)
    reveal_type(f_1), reveal_type(f_2)

    g_1 = lit(g)
    g_2 = lit_intuitive(g)
    reveal_type(g_1), reveal_type(g_2)

    h_1 = lit(h)
    h_2 = lit_intuitive(h)
    reveal_type(h_1), reveal_type(h_2)

    i_1 = lit(i)
    i_2 = lit_intuitive(i)
    reveal_type(i_1), reveal_type(i_2)

    j_1 = lit(j)
    j_2 = lit_intuitive(j)
    reveal_type(j_1), reveal_type(j_2)

    k_1 = lit(k)
    k_2 = lit_intuitive(k)
    reveal_type(k_1), reveal_type(k_2)

    l_1 = lit(l)
    l_2 = lit_intuitive(l)
    reveal_type(l_1), reveal_type(l_2)

    m_1 = lit(m)
    m_2 = lit_intuitive(m)
    reveal_type(m_1), reveal_type(m_2)

    n_1 = lit(n)
    n_2 = lit_intuitive(n)
    reveal_type(n_1), reveal_type(n_2)

Outputs

mypy

>>> mypy narwhals/_ibis/t.py 
narwhals/_ibis/t.py:74: note: Revealed type is "Any"
narwhals/_ibis/t.py:78: note: Revealed type is "Any" 
narwhals/_ibis/t.py:82: note: Revealed type is "Any" 
narwhals/_ibis/t.py:86: note: Revealed type is "Any" 
narwhals/_ibis/t.py:90: note: Revealed type is "Any" 
narwhals/_ibis/t.py:94: note: Revealed type is "Any" 
narwhals/_ibis/t.py:98: note: Revealed type is "Any" 
narwhals/_ibis/t.py:102: note: Revealed type is "Any"
narwhals/_ibis/t.py:106: note: Revealed type is "Any"
narwhals/_ibis/t.py:110: note: Revealed type is "Any"
narwhals/_ibis/t.py:114: note: Revealed type is "Any"
narwhals/_ibis/t.py:118: note: Revealed type is "Any"
narwhals/_ibis/t.py:122: note: Revealed type is "Any"
narwhals/_ibis/t.py:126: note: Revealed type is "Any"
Success: no issues found in 1 source file

pyright

>>> pyright narwhals/_ibis/t.py
narwhals/_ibis/t.py
  narwhals/_ibis/t.py:74:17 - information: Type of "a_1" is "BooleanScalar"
  narwhals/_ibis/t.py:74:35 - information: Type of "a_2" is "BooleanScalar"
  narwhals/_ibis/t.py:78:17 - information: Type of "b_1" is "BooleanScalar"
  narwhals/_ibis/t.py:78:35 - information: Type of "b_2" is "BooleanScalar"
  narwhals/_ibis/t.py:82:17 - information: Type of "c_1" is "BooleanScalar"
  narwhals/_ibis/t.py:82:35 - information: Type of "c_2" is "BooleanScalar"
  narwhals/_ibis/t.py:86:17 - information: Type of "d_1" is "BooleanScalar"
  narwhals/_ibis/t.py:86:35 - information: Type of "d_2" is "BooleanScalar"
  narwhals/_ibis/t.py:90:17 - information: Type of "e_1" is "IntegerScalar"
  narwhals/_ibis/t.py:90:35 - information: Type of "e_2" is "IntegerScalar"
  narwhals/_ibis/t.py:94:17 - information: Type of "f_1" is "IntegerScalar"
  narwhals/_ibis/t.py:94:35 - information: Type of "f_2" is "IntegerScalar"
  narwhals/_ibis/t.py:98:17 - information: Type of "g_1" is "IntegerScalar"
  narwhals/_ibis/t.py:98:35 - information: Type of "g_2" is "IntegerScalar"
  narwhals/_ibis/t.py:102:17 - information: Type of "h_1" is "IntegerScalar"
  narwhals/_ibis/t.py:102:35 - information: Type of "h_2" is "IntegerScalar"
  narwhals/_ibis/t.py:106:17 - information: Type of "i_1" is "FloatingScalar | IntegerScalar"
  narwhals/_ibis/t.py:106:35 - information: Type of "i_2" is "IntegerScalar"
  narwhals/_ibis/t.py:110:17 - information: Type of "j_1" is "FloatingScalar | IntegerScalar"
  narwhals/_ibis/t.py:110:35 - information: Type of "j_2" is "FloatingScalar"
  narwhals/_ibis/t.py:114:17 - information: Type of "k_1" is "StringScalar"
  narwhals/_ibis/t.py:114:35 - information: Type of "k_2" is "StringScalar"
  narwhals/_ibis/t.py:118:17 - information: Type of "l_1" is "IntegerScalar"
  narwhals/_ibis/t.py:118:35 - information: Type of "l_2" is "IntegerScalar"
  narwhals/_ibis/t.py:122:17 - information: Type of "m_1" is "Scalar"
  narwhals/_ibis/t.py:122:35 - information: Type of "m_2" is "Scalar"
  narwhals/_ibis/t.py:126:17 - information: Type of "n_1" is "Unknown"
  narwhals/_ibis/t.py:126:35 - information: Type of "n_2" is "Unknown"
0 errors, 0 warnings, 28 informations

Important

pyright does resolve i: int and j: float to different overloads!!! 🥳

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ibis doesn't have py.typed so mypy ignores it completely afaiu

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed!

@dangotbanned dangotbanned requested a review from FBruzzesi August 14, 2025 12:51
Copy link
Copy Markdown
Member

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Thanks for the adjustment!

@dangotbanned dangotbanned merged commit 5ea6ee4 into main Aug 14, 2025
30 of 31 checks passed
@dangotbanned dangotbanned deleted the ibis-typing-fixes-1 branch August 14, 2025 13:42
@dangotbanned dangotbanned mentioned this pull request Aug 8, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ibis Issue is related to ibis backend internal typing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants